Solving #23

There are additional places where the split function is currently accepting undef input from missing facts:

• https://github.com/covermymeds/puppet-gluster/blob/master/manifests/volume.pp#L114
• https://github.com/covermymeds/puppet-gluster/blob/master/manifests/volume.pp#L168

If you build a cluster to establish peers, and then add a volume, the above should work because the facts will be populated. If you try to set up a brand-new installation that includes a volume definition, these will fail because the facts are empty. Obviously, it's desirable to be able to build a complete cluster in one shot. (That is to say: one shot at defining the configuration. Multiple Puppet runs will be required; but what we don't want to do is define an initial config, run Puppet, apply some more config, re-run Puppet, and then have a cluster.)

I've got it working with the following changes:

L105:

    if $::gluster_peer_list != undef and $::gluster_volume_list != undef {
      $minimal_requirements = true
    } else {
      $minimal_requirements = false
    }
    if $::gluster_volume_list != undef and member( split( $::gluster_volume_list, ',' ), $title ) {
      $already_exists = true
    } else {
      $already_exists = false
    }

    if $minimal_requirements and $already_exists == false {

And then on (the new) L175, modify the else to:

    } elsif $already_exists {

Can you please double-check my work here to help make sure we're not missing anything?

@tux-o-matic another way to tackle this is to update the custom fact to actually return an empty string for gluster_peer_list and gluster_volume_list.

Doing so causes problems on line 142 of volume.pp, at the least, so some additional logic in the manifest will be necessary.

Other problems might be lurking elsewhere with empty strings versus undefined facts, of course.

And if we're changing custom facts, we might as well use structured facts to return native hashes and arrays, in order to avoid a lot of the contortions we go through in the manifest to convert comma-separated lists into hashes and arrays.

@tux-o-matic I've just transferred this module to the Puppet Community organization. Your PR was transferred, too; but please update your bookmarks and your .git/config as necessary. Thanks!

I won't have an environment to test a new approach with volume creation from scratch for a few days. Which should be aimed for? Validating my PR with your additions in volume.pp or fixing the facts to never return undef?

With my latest batch of commits, I've been able to resolve issues blocking the creation of a Gluster volume from scratch (no pre-existing Gluster volume in the pool).

@skpy There was a middle ground to find between my first commits and your latest suggestion as the Fact 'gluster_volume_list' can be undef when no Gluster volumes exist. So the Fact being defined should not be a requirement to create a volume or else the first volume can never be created by this module.
So now it looks like this:

    if $::gluster_peer_list != undef {
      $minimal_requirements = true
    } else {
      $minimal_requirements = false
    }

    if $::gluster_volume_list != undef and member( split( $::gluster_volume_list, ',' ), $title ) {
      $already_exists = true
    } else {
      $already_exists = false
    }

    if $minimal_requirements and $already_exists == false {

If it makes sense to you then the checks can be further simplified by having '$::gluster_peer_list != undef' replace altogether the $minimal_requirements bool.

Some changes were also needed to peer.pp

I also included some corrections from #31 to be able to complete Puppet runs without errors.

I also created more Gluster volumes on the same nodes/pool following the first one done from scratch by Puppet and can confirm that the new code does not interfere with this original feature.

@skpy Anything holding this PR from being merged?

@tux-o-matic please squash.

CI is failing all of a sudden. Is that due to changes on the repository? Maybe time to update RPM version in tests and maybe YUM repo base.

@jyaworski Squashed.

I'm going to merge. The failures are due to strict variable nonsense in the tests. Someone has to get around to refactoring the tests....